Generate nullable as type alias and resolve recursive reference in union #2989
Conversation
| } | null; | ||
|
|
||
| // @public | ||
| export type ErrorModel_1 = { |
There was a problem hiding this comment.
@maorleger this PR can fix the generation error, but it will have duplicate models issue because of this Azure/typespec-azure#1677 (comment) And @tadelesh will fix that after the holiday in tcgc. will you be okay with this fix for now and remove the duplicated models while you prepare the PR ? And we will fix this in emitter side after the holiday.
If you agree with this, I will try to release a new emitter with the current fix before the holiday.
There was a problem hiding this comment.
@qiaozha thanks for the heads up - this will be fine for me as KeyVault's generated code is not directly exported. We have a convenience layer and can smooth over any duplication as this. Thank you!
qiaozha
changed the title
|
|
||
| // @public | ||
| export type Stop = string | string[]; | ||
| export type Prompt_1 = string | string[] | number[] | number[][]; |
There was a problem hiding this comment.
i was thinking if we should delivery an alpha verion for keyvalut first. the correct fix without xxx_1 should be non-breaking, but this pr would introduce a breaking.
There was a problem hiding this comment.
breaking only exists when there's a real release, and as far as I know keyvault is the only one that has this issue and is going to release, and @maorleger already said they will not release with this duplicate _N suffix. And it seems like an easy fix and can be easily caught during the review stage.
However, any services that are using recursive self reference union would have the same generation crash issue in pipeline. Compare with the generation crash there, we then spend resources to investigate during this period, I prefer to have a new release out with the latest tag.
/cc @joheredi @xirzec Could you kindly keep that in mind while you review the servics onboarding PR? Thanks.
|
|
||
| // @public | ||
| export type Stop = string | string[]; | ||
| export type Prompt_1 = string | string[] | number[] | number[][]; |
There was a problem hiding this comment.
is this because tcgc wrongly tagged this union type as name type but it should be an anoymous one?
There was a problem hiding this comment.
yeah, see issue here Azure/typespec-azure#2133
| // We already know it's not a model type | ||
| const variantType = getSchemaForType(dpgContext, variant.type, { | ||
| ...options, | ||
| needRef: isAnonymousModelType(variant.type) ? false : true |
There was a problem hiding this comment.
Is it because we change the needRef condition so we fix the max stack issue in RLC? I was just thinking if this is a generic way to fix this and if there is other cases we didn't consider. Can we verify it?
union A {
null,
A
}
There was a problem hiding this comment.
tcgc has a problem handle the case too Azure/typespec-azure#2132
fixes #2993